Skip to content
This repository has been archived by the owner on Apr 22, 2019. It is now read-only.

Save in gist #138

Open
wants to merge 6 commits into
base: redux
Choose a base branch
from
Open

Save in gist #138

wants to merge 6 commits into from

Conversation

tianyiii
Copy link

@tianyiii tianyiii commented Jun 8, 2017

create save button that save spec as anonymous gist

@domoritz domoritz requested a review from mathisonian June 13, 2017 23:13
@willium
Copy link
Member

willium commented Jun 14, 2017

sweet!

* redux: (35 commits)
  fixed setState warning (#151)
  Only save spec on parse
  Copy schema from docs instead of re-building vega.
  Update examples and images
  fixed router
  Add debounce
  Don’t format carefully crafted specs. cc @kanitw
  Open idl homepage in new window
  Enable escape on modal, larger modal
  Update CSS for DOM bindings. Fixes #144
  Fix all the editor resize issues, make it much faster because we don’t redraw the editor any more, and also fix #145
  Clean up header
  Fix warning about history push
  Show overflow so that tooltips work
  Clean up css
  Large modal
  Remove debugger that doesn’t work
  Clean up deployment
  Delete lockfile
  Fix prop warning
  ...
@@ -0,0 +1,28 @@
module['exports'] = function myService (hook) {

var github = require('octonode');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could use const here.


var github = require('octonode');

var filename = hook.params.filename;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inline vars that are only used once


var ghgist = client.gist();

ghgist.create({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix indentation

const getSpecFromGistURL = (gistUrl, mode, cb) => {
let prefix = 'https://hook.io/tianyiii/vegaeditor/';
let hookUrl = prefix + mode + '/'
+ gistUrl.substring(gistUrl.indexOf('.com/') + '.com/'.length);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this doing? Could we use a regex instead? May be more readable.

}
let username = arrayNames[1];
let id = arrayNames[2];
hashHistory.push('/gist/' + mode +'/' + username + '/' + id);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use string templates

50% { opacity:0; }
100% { opacity:1; }
}
@-o-keyframes flickerAnimation{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need the prefixing or does some plugin take care of it?

}).then((value) => {
let arrayNames = value[filename].raw_url.split('/');
let id = arrayNames[4];
hashHistory.push('/gist/' + this.props.mode +'/anonymous/' + id);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use template strings

const saveGist = (
<div>
<div className='save-gist-content'>
Saving gist... please wait...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Saving spec in gist. Please wait..."

@@ -269,6 +278,25 @@ export default class Header extends React.Component {
</Portal>

<Portal
closeOnOutsideClick={true}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The outside click doesn't work. Just remove it.

@domoritz
Copy link
Member

@mathisonian @tianyiii, should we try to get this in or wait and resolve conflicts later?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants